Skip to content

Conversation

juliusmh
Copy link
Contributor

No description provided.

@juliusmh juliusmh force-pushed the jmh/validation_scheme_pflag_json branch from 70453fd to 06cb330 Compare July 31, 2025 11:47
Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why you're removing the fmt.Stringer assertion.

@aknuds1 aknuds1 requested a review from Copilot July 31, 2025 11:56
Copilot

This comment was marked as outdated.

@juliusmh juliusmh force-pushed the jmh/validation_scheme_pflag_json branch from 06cb330 to 8454829 Compare July 31, 2025 12:00
@aknuds1 aknuds1 dismissed their stale review July 31, 2025 12:02

Stale.

@aknuds1 aknuds1 self-requested a review July 31, 2025 12:02
…er/Unmarshaler interfaces

Signed-off-by: Julius Hinze <[email protected]>
@juliusmh juliusmh force-pushed the jmh/validation_scheme_pflag_json branch from 8454829 to 2e9eb8f Compare August 1, 2025 08:18
@aknuds1 aknuds1 requested a review from Copilot August 25, 2025 07:41
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements the pflag.Value and json.Marshaler/Unmarshaler interfaces for the ValidationScheme type, enabling it to be used as a command-line flag and for JSON serialization/deserialization.

  • Adds JSON marshaling/unmarshaling methods for ValidationScheme
  • Implements pflag.Value interface with Set() and Type() methods
  • Refactors existing YAML unmarshaling to use the new Set() method

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
model/metric.go Implements JSON and pflag interfaces, refactors YAML unmarshaling
model/metric_test.go Adds comprehensive test coverage for JSON marshaling and pflag Set method

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@aknuds1 aknuds1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, except for a couple of superfluous casts.

Signed-off-by: Arve Knudsen <[email protected]>
Signed-off-by: Arve Knudsen <[email protected]>
@aknuds1 aknuds1 merged commit 053ba9a into prometheus:main Aug 25, 2025
8 checks passed
juliusmh added a commit to grafana/mimir that referenced this pull request Aug 26, 2025
…etheus/common/model.ValidationScheme (#12505)

<!--  Thanks for sending a pull request!  Before submitting:

1. Read our CONTRIBUTING.md guide
2. Rebase your PR if it gets out of sync with main
-->

#### What this PR does

Removes unnecessary `validation.ValidationSchemeValue` because
prometheus/common#807 was merged.


#### Checklist

- [x] Tests updated.
- [ ] Documentation added.
- [ ] `CHANGELOG.md` updated - the order of entries should be
`[CHANGE]`, `[FEATURE]`, `[ENHANCEMENT]`, `[BUGFIX]`. If changelog entry
is not needed, please add the `changelog-not-needed` label to the PR.
- [ ]
[`about-versioning.md`](https://github.com/grafana/mimir/blob/main/docs/sources/mimir/configure/about-versioning.md)
updated with experimental features.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants